fix($rootScope): deallocate $$listeners array when empty #16277
Conversation
There was a problem hiding this comment.
TBH, I am not super keen on this change. It will make destroying scopes and adding a listener for an event whose listeners were previously removed (which is exactly what caused the leak in #16135) a little less efficient. And both of these operations are quite frequent. On the other side, it will save one empty array worth of memory for the rare case that an event is subscribed and then unsubscribed to (forever) 😕
Did I miss something?
| @@ -1375,6 +1374,7 @@ function $RootScopeProvider() { | |||
|
|
|||
| if (current.$$listenerCount[name] === 0) { | |||
| delete current.$$listenerCount[name]; | |||
| delete current.$$listeners[name]; | |||
There was a problem hiding this comment.
One issue brought up is that $$listeners and $$listenerCount often do not have the same lifetime. $$listenerCount will reach 0 only when this scope and all child scopes have no listeners. I think that might convince me that this PR is no good as-is. But I still thinking cleaning up $$listeners when empty might be a decent idea. TBD...
jbedard
force-pushed
the
listener-removal
branch
from
b442535 to
c9e18c2
Compare
|
Now that the array is resized on event removal I don't think this is as important because the array will actually shrink when events are removed. I'm going to close this for now but let me know if anyone disagrees or wants to look into this more... |
I think making
$$listenerCount[name]and$$listeners[name]behave the same makes sense. Today the$$listenersarray might also grow and never be cleaned up unless an event is fired (#16135), however if there are no listeners then even firing an event won't clean it up, but this will in that case.